Should we maybe also mark the struct as #[non_exhaustive]? @Lorak-mmk @wprzytula WDYT?

Or introduce pub getters instead of pubifying the fields.

Pub getters would prevent matching on errors

Pub getters would prevent matching on errors

Well, you still could pattern match down until ConnectionSetupRequestError, and then you could indeed not pattern match deeper. Would it be a serious limitation?

Well, consider this patch to upgrade our project to the latest release of scylla-rust-driver:
https://github.com/shotover/shotover-proxy/pull/1840/files#diff-3c1b291110379020fb6bc7d1644e2e0027997d5243b1c111d1a927a577f9a2c9

The patch uses this PR to scylla-rust-driver that makes the field public allowing this to work:

    match err {
        NewSessionError::ConnectionPoolError(ConnectionPoolError::Broken {
            last_connection_error:
                ConnectionError::ConnectionSetupRequestError(ConnectionSetupRequestError {
                    error: ConnectionSetupRequestErrorKind::DbError(DbError::ServerError, err),
                    ..
                }),
        }) => assert_eq!(format!("{err}"), "expected error"),
        _ => panic!("Unexpected error, was {err:?}"),
    }

Using the latest release of scylla-rust-driver we instead need to break it into two separate matches:

    match err {
        NewSessionError::ConnectionPoolError(ConnectionPoolError::Broken {
            last_connection_error: ConnectionError::ConnectionSetupRequestError(err),
        }) => match err.get_error() {
            ConnectionSetupRequestErrorKind::DbError(DbError::ServerError, err) => {
                assert_eq!(format!("{err}"), "expected error")
            }
            _ => panic!("Unexpected error, was {err:?}"),
        },
        _ => panic!("Unexpected error, was {err:?}"),
    }

So the current state of things is usable and enables breaking changes of the internal fields of the struct in the future.
On the other hand I strongly value being able to write a single match rather than having to break it up into two separate matches.

Its ultimately your call.

Imo making the fields public and marking the struct #[non_exhaustive] should be the solution here.
This is consistent with our enums error types, which have tuple variants, so their fields are visible, and so we can add new variants, but not change existing ones.
Here similarly we will be able to add new fields (e.g. we may want to store some context in the future), but won't be able to edit existing fields.

As an exception, I'd keep structs like BrokenConnectionError with private fields. This is because the internal type can't be matched anyway since if first needs to be downcasted, for which we have the API.

So, unless @wprzytula or @muzarski have strong objections, I'd ask you @rukai to add #[non_exhaustive] and then we'll merge it.

So, unless @wprzytula or @muzarski have strong objections, I'd ask you @rukai to add #[non_exhaustive] and then we'll merge it.

I'm fine with this approach

that works for me!
I've pushed the changes.

Pub getters would prevent matching on errors

Well, you still could pattern match down until ConnectionSetupRequestError, and then you could indeed not pattern match deeper. Would it be a serious limitation?